Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Ethernet support for Portenta + Ethernet Shield #81

Merged
merged 16 commits into from
Nov 3, 2022
Merged

Add Ethernet support for Portenta + Ethernet Shield #81

merged 16 commits into from
Nov 3, 2022

Conversation

pennam
Copy link
Contributor

@pennam pennam commented Jul 27, 2022

This PR is based on #41 and #70 and adds ethernet support for Portenta H7. In order to get Ethernet connectivity the vision shield needs to be connected.

Since there is plenty of flash available for Portenta, Ethernet connectivity is always included in the build together with WiFi support.

Ci is failing because static IP configuration is based on this arduino/ArduinoCore-mbed#526

@pennam pennam requested review from aentinger and facchinm July 27, 2022 07:51
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jul 27, 2022
@pennam pennam mentioned this pull request Jul 27, 2022
Merged
aentinger
aentinger approved these changes Jul 28, 2022
View reviewed changes
Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Good extension of previously laid out pattern 🙇

src/Arduino_EthernetConnectionHandler.cpp Outdated
NetworkConnectionState EthernetConnectionHandler::update_handleInit()
{
if (Ethernet.hardwareStatus() == EthernetNoHardware) {
#if !defined(__AVR__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not explicitly targeted on this PR. But since AVR is not and will not be supported on Arduino IoT Cloud (its not technically feasible), you can drop all those #if !defined(__AVR__)statements.

@arduino-libraries arduino-libraries deleted a comment from github-actions bot Sep 16, 2022
@arduino-libraries arduino-libraries deleted a comment from github-actions bot Sep 16, 2022
@pennam
Copy link
Contributor Author

pennam commented Sep 16, 2022

@manchoz i've updated the PR. This should fix also the reconnection issue. Would you give it a spin?

@manchoz
Copy link
Contributor

manchoz commented Sep 16, 2022

👋 @pennam it works perfectly!

LGTM

@github-actions
Copy link

Memory usage change @ 2462ec0

Board flash % RAM for global variables %
arduino:mbed:envie_m7 🔺 +1896 - +1896 +0.24 - +0.24 🔺 +288 - +288 +0.06 - +0.06
arduino:mbed_nano:nanorp2040connect 🔺 +4 - +4 0.0 - 0.0 🔺 +4 - +4 0.0 - 0.0
arduino:mbed_nicla:nicla_vision 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:mkr1000 💚 -8 - -8 -0.0 - -0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrgsm1400 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrnb1500 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrwan1300 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrwan1310 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrwifi1010 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:nano_33_iot 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
esp32:esp32:esp32 0 - 0 0.0 - 0.0 🔺 +8 - +8 0.0 - 0.0
esp8266:esp8266:huzzah 🔺 +16 - +16 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/ConnectionHandlerDemo
flash		 % examples/ConnectionHandlerDemo
RAM for global variables		 %
arduino:mbed:envie_m7 1896 0.24 288 0.06
arduino:mbed_nano:nanorp2040connect 4 0.0 4 0.0
arduino:mbed_nicla:nicla_vision 0 0.0 0 0.0
arduino:samd:mkr1000 -8 -0.0 4 0.01
arduino:samd:mkrgsm1400 8 0.0 4 0.01
arduino:samd:mkrnb1500 8 0.0 4 0.01
arduino:samd:mkrwan1300 8 0.0 4 0.01
arduino:samd:mkrwan1310 8 0.0 4 0.01
arduino:samd:mkrwifi1010 8 0.0 4 0.01
arduino:samd:nano_33_iot 8 0.0 4 0.01
esp32:esp32:esp32 0 0.0 8 0.0
esp8266:esp8266:huzzah 16 0.0 0 0.0
Click for full report CSV 
Board,examples/ConnectionHandlerDemo<br>flash,%,examples/ConnectionHandlerDemo<br>RAM for global variables,%
arduino:mbed:envie_m7,1896,0.24,288,0.06
arduino:mbed_nano:nanorp2040connect,4,0.0,4,0.0
arduino:mbed_nicla:nicla_vision,0,0.0,0,0.0
arduino:samd:mkr1000,-8,-0.0,4,0.01
arduino:samd:mkrgsm1400,8,0.0,4,0.01
arduino:samd:mkrnb1500,8,0.0,4,0.01
arduino:samd:mkrwan1300,8,0.0,4,0.01
arduino:samd:mkrwan1310,8,0.0,4,0.01
arduino:samd:mkrwifi1010,8,0.0,4,0.01
arduino:samd:nano_33_iot,8,0.0,4,0.01
esp32:esp32:esp32,0,0.0,8,0.0
esp8266:esp8266:huzzah,16,0.0,0,0.0

@arduino-libraries arduino-libraries deleted a comment from github-actions bot Oct 20, 2022
@pennam
Copy link
Contributor Author

pennam commented Oct 20, 2022

@manchoz @aentinger sorry to bother you again with this PR. I have done some changes :

1026b98 Since eth connection is available on multiple boards for portenta (breakout, machine control...) it makes no reason to add a specific define for each one.

1f3b9a5 subnet is not the correct name to use so i've renamed it into netmask.

b53dd95 This one is to be able to create a simpler template for cloud sketches.

2462ec0 And this is for const correctness

manchoz
manchoz approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@manchoz manchoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aentinger
Copy link
Contributor

LGTM 👍

@github-actions
Copy link

github-actions bot commented Nov 2, 2022

Memory usage change @ 4e68980

Board flash % RAM for global variables %
arduino:mbed:envie_m7 🔺 +1808 - +1808 +0.23 - +0.23 🔺 +376 - +376 +0.07 - +0.07
arduino:mbed_nano:nanorp2040connect 🔺 +4 - +4 0.0 - 0.0 🔺 +4 - +4 0.0 - 0.0
arduino:mbed_nicla:nicla_vision 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:mkr1000 💚 -8 - -8 -0.0 - -0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrgsm1400 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrnb1500 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrwan1300 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrwan1310 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:mkrwifi1010 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
arduino:samd:nano_33_iot 🔺 +8 - +8 0.0 - 0.0 🔺 +4 - +4 +0.01 - +0.01
esp32:esp32:esp32 0 - 0 0.0 - 0.0 🔺 +8 - +8 0.0 - 0.0
esp8266:esp8266:huzzah 🔺 +16 - +16 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/ConnectionHandlerDemo
flash		 % examples/ConnectionHandlerDemo
RAM for global variables		 %
arduino:mbed:envie_m7 1808 0.23 376 0.07
arduino:mbed_nano:nanorp2040connect 4 0.0 4 0.0
arduino:mbed_nicla:nicla_vision 0 0.0 0 0.0
arduino:samd:mkr1000 -8 -0.0 4 0.01
arduino:samd:mkrgsm1400 8 0.0 4 0.01
arduino:samd:mkrnb1500 8 0.0 4 0.01
arduino:samd:mkrwan1300 8 0.0 4 0.01
arduino:samd:mkrwan1310 8 0.0 4 0.01
arduino:samd:mkrwifi1010 8 0.0 4 0.01
arduino:samd:nano_33_iot 8 0.0 4 0.01
esp32:esp32:esp32 0 0.0 8 0.0
esp8266:esp8266:huzzah 16 0.0 0 0.0
Click for full report CSV 
Board,examples/ConnectionHandlerDemo<br>flash,%,examples/ConnectionHandlerDemo<br>RAM for global variables,%
arduino:mbed:envie_m7,1808,0.23,376,0.07
arduino:mbed_nano:nanorp2040connect,4,0.0,4,0.0
arduino:mbed_nicla:nicla_vision,0,0.0,0,0.0
arduino:samd:mkr1000,-8,-0.0,4,0.01
arduino:samd:mkrgsm1400,8,0.0,4,0.01
arduino:samd:mkrnb1500,8,0.0,4,0.01
arduino:samd:mkrwan1300,8,0.0,4,0.01
arduino:samd:mkrwan1310,8,0.0,4,0.01
arduino:samd:mkrwifi1010,8,0.0,4,0.01
arduino:samd:nano_33_iot,8,0.0,4,0.01
esp32:esp32:esp32,0,0.0,8,0.0
esp8266:esp8266:huzzah,16,0.0,0,0.0

@pennam pennam changed the title Add Ethernet support for Portenta + Vision Shield Add Ethernet support for Portenta + Ethernet Shield Nov 2, 2022
@pennam pennam merged commit 04af28b into arduino-libraries:master Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aentinger aentinger aentinger approved these changes

@manchoz manchoz manchoz approved these changes

@facchinm facchinm Awaiting requested review from facchinm

Assignees
No one assigned
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pennam @manchoz @aentinger @per1234